Refactor: use Arc<[PathBuf]> for AllowedPathResolver by Sewer56 · Pull Request #2 · Reloaded-Project/ReloadedCode

Sewer56 · 2026-01-15T21:53:41Z

Summary

Changed AllowedPathResolver::allowed_paths from Vec<PathBuf> to Arc<[PathBuf]>
Updated new() to accept generic impl IntoIterator<Item = impl AsRef<Path>>, moving the conversion logic into the resolver
Removed duplicated path conversion boilerplate from all 5 allowed tool constructors (read, write, edit, glob, grep)

Benefits

Cheap Clone: Arc enables O(1) cloning instead of deep-copying all paths
Signals immutability: The paths are set at construction and never mutated
DRY: Eliminates 5 copies of identical conversion code

Initial implementation of LLM coding tools

Move path-to-Vec<PathBuf> conversion into AllowedPathResolver::new(), eliminating duplicated boilerplate in all 5 allowed tool constructors. Using Arc<[PathBuf]> instead of Vec enables cheap Clone and signals immutability.

coderabbitai · 2026-01-15T21:55:15Z

Walkthrough

The change refactors AllowedPathResolver to store allowed_paths as Arc<[PathBuf]> instead of Vec<PathBuf>, and updates its constructors to accept impl IntoIterator<Item = impl AsRef<Path>>, canonicalizing inputs into an Arc<[PathBuf]>. The resolver's resolve iteration uses self.allowed_paths.iter(). Downstream modules (glob, grep, read, write, edit) now forward the allowed_paths iterator directly to the resolver and adjust imports from PathBuf to Path.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: refactoring AllowedPathResolver to use Arc<[PathBuf]> instead of Vec.
Description check	✅ Passed	The description provides a clear summary of changes, affected files, and benefits, exceeding the minimal template requirements.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch refactor/arc-allowed-paths

🧹 Recent nitpick comments

src/llm-coding-tools-core/src/path/allowed.rs (1)
74-81: Consider documenting the semantic safety concern more precisely.

The # Safety heading typically implies memory safety (i.e., unsafe code). Since this function is safe in the Rust sense, consider renaming the section to # Correctness or # Important to avoid confusion with unsafe fn conventions.
📝 Suggested documentation clarification
     /// Use this when paths are known to be valid and canonicalized,
     /// skipping the filesystem check.
     ///
-    /// # Safety
+    /// # Correctness
     ///
     /// Caller must ensure paths are actually canonical. Using non-canonical
     /// paths may allow path traversal attacks.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 397a9a4 and 026d084.

📒 Files selected for processing (6)

src/llm-coding-tools-core/src/path/allowed.rs
src/llm-coding-tools-rig/src/allowed/edit.rs
src/llm-coding-tools-rig/src/allowed/glob.rs
src/llm-coding-tools-rig/src/allowed/grep.rs
src/llm-coding-tools-rig/src/allowed/read.rs
src/llm-coding-tools-rig/src/allowed/write.rs

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.rs

📄 CodeRabbit inference engine (src/AGENTS.md)

src/**/*.rs: Preallocate collections with String::with_capacity() and Vec::with_capacity() when size is known or estimable
Use power-of-two sizes for allocator efficiency via .next_power_of_two() when allocating collections
Prefer &str and &[T] returns over owned types when the lifetime allows
Use Cow<'_, str> for conditional ownership patterns, such as with String::from_utf8_lossy()
Use &'static str for compile-time constant strings
Reuse buffers by calling .clear() and reusing Vec/String instead of reallocating
Use const generics for compile-time branching (e.g., <const LINE_NUMBERS: bool>) as a zero-cost abstraction
Use #[inline] attribute on small, hot-path functions to enable compiler inlining
Prefer core crate over std where possible (e.g., core::mem over std::mem)
Stream data instead of loading entire files into memory when possible
Use the memchr crate for fast byte searching instead of manual iteration
Keep Rust modules under 500 lines (excluding tests); split larger modules into separate files
Place use statements inside functions only for #[cfg] conditional compilation; use module-level imports otherwise
Document all public items with /// doc comments and add examples in documentation where helpful
Focus comments on 'why' not 'what' - avoid restating code logic in comments
Use rustdoc link syntax [TypeName] instead of backticks for type references in documentation

Files:

src/llm-coding-tools-rig/src/allowed/write.rs
src/llm-coding-tools-rig/src/allowed/edit.rs
src/llm-coding-tools-rig/src/allowed/read.rs
src/llm-coding-tools-core/src/path/allowed.rs
src/llm-coding-tools-rig/src/allowed/glob.rs
src/llm-coding-tools-rig/src/allowed/grep.rs

🧠 Learnings (3)

📚 Learning: 2026-01-15T09:19:17.808Z

Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-15T09:19:17.808Z
Learning: Applies to src/**/*.rs : Prefer `&str` and `&[T]` returns over owned types when the lifetime allows

Applied to files:

src/llm-coding-tools-rig/src/allowed/write.rs
src/llm-coding-tools-rig/src/allowed/edit.rs
src/llm-coding-tools-rig/src/allowed/read.rs
src/llm-coding-tools-core/src/path/allowed.rs
src/llm-coding-tools-rig/src/allowed/glob.rs
src/llm-coding-tools-rig/src/allowed/grep.rs

📚 Learning: 2026-01-15T09:19:17.808Z

Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-15T09:19:17.808Z
Learning: Applies to src/**/*.rs : Prefer `core` crate over `std` where possible (e.g., `core::mem` over `std::mem`)

Applied to files:

src/llm-coding-tools-rig/src/allowed/edit.rs

📚 Learning: 2026-01-15T09:19:17.808Z

Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-15T09:19:17.808Z
Learning: Applies to src/**/*.rs : Place `use` statements inside functions only for `#[cfg]` conditional compilation; use module-level imports otherwise

Applied to files:

src/llm-coding-tools-rig/src/allowed/glob.rs
src/llm-coding-tools-rig/src/allowed/grep.rs

🧬 Code graph analysis (6)

src/llm-coding-tools-rig/src/allowed/write.rs (1)

src/llm-coding-tools-core/src/path/allowed.rs (2)

new (45-63)

allowed_paths (84-86)

src/llm-coding-tools-rig/src/allowed/edit.rs (1)

src/llm-coding-tools-core/src/path/allowed.rs (2)

new (45-63)

allowed_paths (84-86)

src/llm-coding-tools-rig/src/allowed/read.rs (1)

src/llm-coding-tools-core/src/path/allowed.rs (2)

new (45-63)

allowed_paths (84-86)

src/llm-coding-tools-core/src/path/allowed.rs (5)

src/llm-coding-tools-rig/src/allowed/edit.rs (1)

new (35-39)

src/llm-coding-tools-rig/src/allowed/glob.rs (1)

new (29-33)

src/llm-coding-tools-rig/src/allowed/grep.rs (1)

new (44-48)

src/llm-coding-tools-rig/src/allowed/read.rs (1)

new (46-50)

src/llm-coding-tools-rig/src/allowed/write.rs (1)

new (29-33)

src/llm-coding-tools-rig/src/allowed/glob.rs (1)

src/llm-coding-tools-core/src/path/allowed.rs (2)

new (45-63)

allowed_paths (84-86)

src/llm-coding-tools-rig/src/allowed/grep.rs (1)

src/llm-coding-tools-core/src/path/allowed.rs (2)

new (45-63)

allowed_paths (84-86)

🔇 Additional comments (15)

src/llm-coding-tools-core/src/path/allowed.rs (5)

5-6: LGTM!

The imports are correctly updated to support the new Arc<[PathBuf]> storage and the generic impl AsRef<Path> parameter.

34-37: LGTM!

Using Arc<[PathBuf]> is a good choice here - it enables O(1) cloning of the resolver while signaling that the paths are immutable after construction. The field documentation accurately describes the contents.

45-63: LGTM!

The constructor now accepts any IntoIterator<Item = impl AsRef<Path>>, which is more ergonomic and eliminates the need for callers to pre-convert paths. The collect() into Result<Arc<[PathBuf]>, _> correctly leverages FromIterator to build the arc-slice directly.

83-86: LGTM!

Returning &[PathBuf] is correct and aligns with the coding guideline to prefer &[T] returns over owned types. The Arc<[PathBuf]> automatically derefs to &[PathBuf].

94-94: LGTM!

Using self.allowed_paths.iter() is the idiomatic way to iterate over an Arc<[T]>.

src/llm-coding-tools-rig/src/allowed/write.rs (2)

10-10: LGTM!

Import correctly updated to Path since PathBuf is no longer needed in this module.

29-33: LGTM!

The constructor now correctly forwards the iterator directly to AllowedPathResolver::new, eliminating the intermediate Vec<PathBuf> allocation. This is cleaner and more efficient.

src/llm-coding-tools-rig/src/allowed/read.rs (2)

10-10: LGTM!

Import correctly updated to Path.

46-49: LGTM!

The constructor correctly forwards the iterator to the resolver, consistent with the other tool implementations.

src/llm-coding-tools-rig/src/allowed/grep.rs (2)

11-11: LGTM!

Import correctly updated to Path.

44-48: LGTM!

The constructor correctly forwards the iterator to the resolver, maintaining consistency with the other tool implementations in this PR.

src/llm-coding-tools-rig/src/allowed/glob.rs (2)

10-10: LGTM!

Import correctly updated to Path.

29-33: LGTM!

The constructor correctly forwards the iterator to the resolver. All five tool modules now consistently delegate path handling to AllowedPathResolver::new without intermediate allocations.

src/llm-coding-tools-rig/src/allowed/edit.rs (2)

11-11: Import update looks good.

This aligns with the new AsRef<Path>-based signature.

35-38: Constructor now cleanly forwards allowed paths.

Nice simplification and keeps conversion centralized in the resolver.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Sewer56 · 2026-01-15T21:57:08Z

@coderabbitai full-review

coderabbitai · 2026-01-15T21:57:20Z

@Sewer56: I'll perform a full review of all the changes in this PR.

✅ Actions performed

Full review triggered.

Refactor: use Arc<[PathBuf]> for AllowedPathResolver

Sewer56 added 2 commits January 15, 2026 09:18

Merge pull request #1 from Sewer56/impl-initial-tools

8406a4f

Initial implementation of LLM coding tools

Refactor: use Arc<[PathBuf]> for AllowedPathResolver

026d084

Move path-to-Vec<PathBuf> conversion into AllowedPathResolver::new(), eliminating duplicated boilerplate in all 5 allowed tool constructors. Using Arc<[PathBuf]> instead of Vec enables cheap Clone and signals immutability.

Sewer56 merged commit f56786c into impl-initial-tools Jan 15, 2026

Sewer56 deleted the refactor/arc-allowed-paths branch January 15, 2026 22:16

This was referenced Jan 16, 2026

feat: Add llm-coding-tools-serdesai crate #5

Merged

Enhance PreambleBuilder with system_prompt, add_context, and allowed_paths support #17

Merged

Sewer56 added a commit that referenced this pull request Mar 30, 2026

Merge pull request #2 from Sewer56/refactor/arc-allowed-paths

0ed33b5

Refactor: use Arc<[PathBuf]> for AllowedPathResolver

This was referenced Apr 7, 2026

Optimize path resolver performance with early rejection and fast path #90

Merged

Optimize external directory path resolution and add benchmarks #95

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: use Arc<[PathBuf]> for AllowedPathResolver#2

Refactor: use Arc<[PathBuf]> for AllowedPathResolver#2
Sewer56 merged 2 commits intoimpl-initial-toolsfrom
refactor/arc-allowed-paths

Sewer56 commented Jan 15, 2026

Uh oh!

coderabbitai Bot commented Jan 15, 2026 •

edited

Loading

Uh oh!

Sewer56 commented Jan 15, 2026

Uh oh!

coderabbitai Bot commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Sewer56 commented Jan 15, 2026

Summary

Benefits

Uh oh!

coderabbitai Bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

Sewer56 commented Jan 15, 2026

Uh oh!

coderabbitai Bot commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jan 15, 2026 •

edited

Loading